[PWGHF] Species-dependent minimum prong pt cut in skimming#15755
[PWGHF] Species-dependent minimum prong pt cut in skimming#15755Marcellocosti wants to merge 5 commits intoAliceO2Group:masterfrom
Conversation
|
O2 linter results: ❌ 0 errors, |
Please consider the following formatting changes to AliceO2Group#15755
|
Hi @Marcellocosti, thanks! Please before merging this PR, it is important to profile the CPU consumption (for Pb-Pb especially). |
|
Hi @fgrosa, thanks for the suggestion! I just have a doubt: do you want me to profile the CPU consumption related to these code changes or to lower min-pt selections with respect to the ones currently used? Because with this implementation we will be anyway able to run with the min-pt selections used until now with no increase in CPU usage, given that just one check is added. And maybe hyperloop is best suited to monitor the impact of lowering the pt thresholds. |
I would say that at least you should check that without changing the cut (i.e. setting them all as we have now) it does not introduce an overhead. Then, I would suggest also to check what is the impact of lowering even only one since keep in mind that this usecase will be only useful for Pb-Pb, since in pp anyway there is no point in reducing it further. |
|
Okay, I will post here the checks you suggested before marking the PR as ready for review. For the use-cases, I was thinking more of lowering it in OO collisions (from the current 0.4 GeV/c), but of course it would be nice to see if also the Pb-Pb case can be lowered. |
|
Hi @fgrosa, I performed local tests reconstructing 55 Pb-Pb collisions in 0-20% centrality. I performed 3 tests and I report the
Please let me know if this is what you had in mind or if you think additional checks are required. Thanks a lot! |
Thanks @Marcellocosti! This shows that the implementation for the current cuts does not introduce overhead, so it's safe to be merged, but releasing the cut for central Pb-Pb collisions increases x3 the time, so it is prohibitive unless we implement other optimisations. This might instead work for OO however, given the lower combinatorics. |
|
Hi @vkucera, thanks for the feedback, I have implemented your comments! |
Implement Vit comments
| Configurable<LabeledArray<double>> cutsDstarToD0Pi{"cutsDstarToD0Pi", {hf_cuts_presel_dstar::Cuts[0], hf_cuts_presel_dstar::NBinsPt, hf_cuts_presel_dstar::NCutVars, hf_cuts_presel_dstar::labelsPt, hf_cuts_presel_dstar::labelsCutVar}, "D*+->D0pi selections per pT bin"}; | ||
|
|
||
| // Species-differential track min pT selection for 3-prong candidates | ||
| Configurable<LabeledArray<float>> ptProngMin3Prong{"ptProngMin3Prong", {hf_cuts_presel_3prong::ptProngMin, hf_cuts_presel_3prong::NSpecies, 1, hf_cuts_presel_3prong::labelsPtProngMin, hf_cuts_presel_3prong::labelsMinPt}, "Min pT selection for prongs of 3-prong candidates in GeV/c"}; |
There was a problem hiding this comment.
I'm a bit confused. This looks like a declaration of a 2D array, but yours is 1D.
There was a problem hiding this comment.
Hi Vit, this was done to have the entry in hyperloop looking like the current onnxFileNames of the workflow hf-track-index-skim-creator, which seems to me the best layout for this config
There was a problem hiding this comment.
But onnxFileNames is a 2D array.
There was a problem hiding this comment.
I saw that onnxFileNames results in a table with rows containing a fixed particle name + a configurable, wouldn't the current implementation achieve the same result for ptProngMin3Prong? The dpl-config.json files from my local test show that, with this implementation, they have the same structure. Or maybe do you have in mind a cleaner alternative?
There was a problem hiding this comment.
Yes, they result in the same structure but your declaration of the default value does not have the same structure. Look for example at the declaration of selectionsPid which has the same data type as ptProngMin3Prong.






This PR implements the possibility to have different minimum prong pt selections for the HF 3 prong candidates in the
trackIndexSkimCreatorworkflow.